Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Adds fallback to res.text in billing error handler #21904

Merged
merged 3 commits into from
Apr 26, 2024
Merged

fix: Adds fallback to res.text in billing error handler #21904

merged 3 commits into from
Apr 26, 2024

Conversation

xrdt
Copy link
Contributor

@xrdt xrdt commented Apr 26, 2024

Problem

See related sentry error: https://posthog.sentry.io/issues/5257727384/?alert_rule_id=14968077&alert_type=issue&notification_uuid=657b771e-3e30-42fa-9a73-56cbba15ddf8&project=1899813&referrer=slack

We were trying to decode to json some responses that were not json encoded.

Changes

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

raquelmsmith
raquelmsmith approved these changes Apr 26, 2024
View reviewed changes
Copy link
Member

@raquelmsmith raquelmsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the fix for this should be more upstream? Why is billing sending an error back to PostHog that isn't properly formatted?

@xrdt
Copy link
Contributor Author

xrdt commented Apr 26, 2024 •
edited
Loading

It’s a 504 gateway error so I think it’s automatically generated. I don’t think there’s any requirement that responses have to be json encoded. I will try to have the ones that we create fulfill that requirement.

@xrdt xrdt merged commit 5f296b4 into master Apr 26, 2024
97 checks passed
@xrdt xrdt deleted the by/billing-error-handler-json-text branch April 26, 2024 18:56
@sentry-io Sentry.io
Copy link

sentry-io bot commented Apr 26, 2024 •
edited
Loading

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Exception: ('Billing service returned bad status code: 500', 'body:', {'type': 'server_error', 'code': 'erro... /api/billing-v2/ View Issue
  • ‼️ Exception: ('Billing service returned bad status code: 504', 'body:', '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HT... posthog.tasks.usage_report.send_report_to_billi... View Issue
  • ‼️ Exception: ('Billing service returned bad status code: 504', 'body:', '\r\n<title>504 Gateway Ti... /api/billing-v2/ View Issue
  • ‼️ Exception: ('Billing service returned bad status code: 504', 'body:', '\r\n<title>504 Gateway Ti... /api/billing-v2/ View Issue
  • ‼️ Exception: ('Billing service returned bad status code: 502', 'body:', '\r\n<title>502 Bad Gatewa... /api/signup/{invite_id}/ View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raquelmsmith raquelmsmith raquelmsmith approved these changes

@zlwaterfield zlwaterfield Awaiting requested review from zlwaterfield

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xrdt @raquelmsmith